AI automated code review (Gemini 3).

Overall risk: high

Summary:
This PR refactors P2P address handling by introducing a unified OceanNode type, replacing ad-hoc string | Multiaddr[] logic. The connection management in P2pProvider has been significantly streamlined, consolidating PeerStore and DHT searches into a unified flow. However, there are a few critical issues to address: a potential runtime TypeError when falling back to p2p-circuit multiaddrs without a peerId, a logic flaw in getSignature that overrides nonce verification, and a loss of connection resilience (fallback to DHT) if explicitly provided multiaddresses fail to connect.

Comments:
• [ERROR][bug] If nodeUri contains a p2p-circuit multiaddr but no peer ID suffix, peerId will be null. The condition afterPFilter < beforePFilter evaluates to true, and calling peerId.toString() will throw a TypeError: Cannot read properties of null. Use optional chaining here and ensure the fallback object safely matches the NodeP2P interface.
• [ERROR][bug] Returning a pre-computed signature here completely ignores the nonce parameter. Functions that call getSignature (like sendP2pCommand or getPersistentStorageSignaturePayload) generate a fresh nonce = String(Date.now()) and send it alongside the signature. Since the pre-computed signature was signed over a different nonce, verification on the provider node will consistently fail. Pre-computed signatures must be explicitly passed alongside their exact corresponding nonces (as is currently done in fetchNodeLogs) rather than intercepted here.
• [WARNING][architecture] In the previous implementation, if dialing the explicitly provided multiaddr failed, the code fell back to searching the PeerStore/DHT for alternative addresses via dialByPeerId. In this refactored version, if the provided nodeUri contains dialable addresses, hasDialable() is true, and PeerStore/DHT lookups are entirely skipped. If node.dial(dialable) subsequently fails, the error is immediately thrown. This removes connection resilience if an explicitly provided IP is offline but the peer is still reachable on another interface known to the DHT. Consider catching the dial error and allowing a fallback to DHT discovery if direct dialing fails.
• [INFO][style] Using template literal backticks for `multiaddress` is syntactically valid but inconsistent with the 'nodeId' check on the same line. Consider using standard quotes for consistency.

AI automated code review (Gemini 3).

Overall risk: medium

Summary:
This PR introduces excellent structural improvements by adding the OceanNode type and AgentSignature interface. This centralizes pre-signed request validation and drastically improves the usability of agent-based workflows. The P2P connection logic is significantly upgraded, adding robust fallbacks (e.g., P2P-circuit relays). Overall the architecture is much cleaner. However, a few implementation issues need addressing: a header injection bug with the fetch API, a TypeScript interface violation during P2P recursive fallbacks, redundant logic checks, and a potential runtime crash if raw hex signature strings are evaluated as auth tokens.

Comments:
• [ERROR][bug] this.getAuthorization(signerOrAuthToken) returns undefined when the argument does not map to a valid JWT token. When undefined is directly passed as a dictionary value in HeadersInit, the fetch API casts it to the string "undefined". You properly avoided this in computeStop using object spreading; the same safeguard should be applied here.

-        headers: {
-          'Content-Type': 'application/json',
-          Authorization: this.getAuthorization(signerOrAuthToken)
-        },
+        headers: {
+          'Content-Type': 'application/json',
+          ...(this.getAuthorization(signerOrAuthToken) ? { Authorization: this.getAuthorization(signerOrAuthToken) } : {})
+        },

• [WARNING][bug] peerId?.toString() evaluates to string | undefined. Because the NodeP2P interface strictly requires nodeId to be a string, passing undefined here violates the type contract. Provide a fallback empty string to satisfy TypeScript strict mode.

-        return this.getConnection(
-          { nodeId: peerId?.toString(), multiaddress: addrs },
-          signal,
-          true
-        )
+        return this.getConnection(
+          { nodeId: peerId ? peerId.toString() : '', multiaddress: addrs },
+          signal,
+          true
+        )

• [WARNING][bug] Assuming a raw hex string starting with 0x is a valid standalone parameter might cause runtime crashes. Almost all provider methods immediately call getConsumerAddress(signerOrAuthToken), which strictly attempts to decode a string via decodeJwt(signerOrAuthToken). If a user actually passes a raw 0x... string, decodeJwt will throw an exception. Consider removing this fallback check and documenting that callers must use the AgentSignature object for raw signatures instead.

// Example: decodeJwt('0xabc...') throws an error.

• [INFO][style] The isAgentSignature method explicitly checks if the provided argument is an object. Therefore, if typeof evaluates to 'string', it is guaranteed not to be an AgentSignature. The && !isAgentSignature(...) condition is logically redundant.

-    if (typeof signerOrAuthToken === 'string' && !isAgentSignature(signerOrAuthToken)) {
+    if (typeof signerOrAuthToken === 'string') {

• [INFO][style] Identical redundant check as above. It can be safely simplified.

-    if (typeof signerOrAuthToken === 'string' && !isAgentSignature(signerOrAuthToken)) {
+    if (typeof signerOrAuthToken === 'string') {

• [INFO][style] Minor syntax inconsistency: Using backticks for property lookups in an object is unnecessary and unconventional. Standard single quotes match the rest of your logic.

-  if (typeof node === 'object' && ('nodeId' in node || `multiaddress` in node)) {
+  if (typeof node === 'object' && ('nodeId' in node || 'multiaddress' in node)) {

AI automated code review (Gemini 3).

Overall risk: low

Summary:
This PR excellently introduces the OceanNode and AgentSignature abstractions, allowing for pre-signed requests that save network round-trips for nonce fetching. Unifying HTTP and P2P payload patterns is a great architectural win. There is one oversight in BaseProvider's validateDdo signature that breaks the new contract, and a strict TypeScript type mismatch when attempting a circuit relay connection. Otherwise, the code is solid. LGTM!

Comments:
• [ERROR][bug] The parameter signer: Signer was left unchanged in the BaseProvider interface. It should be updated to signerOrAuthToken: signerOrAuthTokenOrSignature to match the underlying P2pProvider and HttpProvider implementations. Otherwise, consumers will encounter TypeScript errors when attempting to pass an AgentSignature here.

   public async validateDdo(
     nodeUri: OceanNode,
     ddo: DDO,
-    signer: Signer,
+    signerOrAuthToken: signerOrAuthTokenOrSignature,
     signal?: AbortSignal
   ): Promise<ValidateMetadata> {
-    return this.getImpl(nodeUri).validateDdo(nodeUri, ddo, signer, signal)
+    return this.getImpl(nodeUri).validateDdo(nodeUri, ddo, signerOrAuthToken, signal)
   }

• [ERROR][bug] Passing null for nodeId violates the NodeP2P interface (which defines nodeId: string). In projects with strictNullChecks enabled, this will cause a TypeScript compilation error. Consider passing an empty string or explicitly casting the object.

         return this.getConnection(
-          { nodeId: peerId ? peerId.toString() : null, multiaddress: addrs },
+          { nodeId: peerId ? peerId.toString() : '', multiaddress: addrs } as NodeP2P,
           signal,
           true
         )

• [INFO][style] The logic to conditionally resolve the nonce and signature (checking if it's an AgentSignature vs manually fetching via getNonce and signing) is duplicated heavily (~20 times) across HttpProvider and P2pProvider. Consider extracting this into a protected helper method (e.g., protected async resolveAuthPayload(...)) to keep the code DRY and easier to maintain.
• [INFO][other] Removing fetchNodeLogs and allowing downloadNodeLogs to seamlessly handle both auto-signed and pre-signed requests via AgentSignature is a fantastic API unification. Great design choice here!

AI automated code review (Gemini 3).

Overall risk: medium

Summary:
This PR introduces excellent structural improvements, cleanly abstracting P2P node types via OceanNode and unifying signature checks via AgentSignature and getSignedCommandParams. The DRY refactoring across endpoints is highly commendable. However, there is a critical logic bug: the strict string checks (designed originally for persistent storage methods) were copied into the shared getSignedCommandParams, which will currently crash all standard endpoints when a JWT AuthToken is used instead of a Signer.

Comments:
• [ERROR][bug] The getSignedCommandParams method throws an error when signerOrAuthToken is a string. Because this method is now shared across many standard HTTP endpoints (like computeStart, downloadNodeLogs, etc.), any API flow using an AuthToken will immediately crash. We need to allow strings here and push the strict validation back to getPersistentStorageSignaturePayload.

@@ -48,7 +48,7 @@
     signal?: AbortSignal,
     providerEndpoints?: any,
     serviceEndpoints?: any
-  ): Promise<{ consumerAddress: string; nonce: string; signature: string }> {
+  ): Promise<{ consumerAddress: string; nonce: string; signature: string | null }> {
     if (!providerEndpoints) providerEndpoints = await this.getEndpoints(nodeUri)
     if (!serviceEndpoints)
       serviceEndpoints = await this.getServiceEndpoints(nodeUri, providerEndpoints)
@@ -59,11 +59,6 @@
         signature: signerOrAuthToken.signature
       }
     }
-    if (typeof signerOrAuthToken === 'string') {
-      throw new Error(
-        'Persistent storage operations require a Signer or AgentSignature (nonce/signature).'
-      )
-    }
 
     const consumerAddress = await this.getConsumerAddress(signerOrAuthToken)
     const nonce = (
@@ -88,14 +83,14 @@
     signal?: AbortSignal,
     providerEndpoints?: any,
     serviceEndpoints?: any
   ): Promise<{ consumerAddress: string; nonce: string; signature: string }> {
     if (typeof signerOrAuthToken === 'string') {
       throw new Error(
         'Persistent storage operations require a Signer or AgentSignature (nonce/signature).'
       )
     }
-    return this.getSignedCommandParams(
+    return (await this.getSignedCommandParams(
       nodeUri,
       signerOrAuthToken,
       command,
       signal,
       providerEndpoints,
       serviceEndpoints
-    )
+    )) as { consumerAddress: string; nonce: string; signature: string }
   }

• [ERROR][bug] Similarly to the HTTP Provider, throwing an error when signature is falsy breaks P2P flows that legitimately rely on AuthTokens (where getSignature returns null by design). You should allow signature: string | null and only strictly enforce its presence in getPersistentStorageSignaturePayload.

@@ -377,21 +377,18 @@
     signerOrAuthToken: signerOrAuthTokenOrSignature,
     command: string,
     signal?: AbortSignal
-  ): Promise<{ consumerAddress: string; nonce: string; signature: string }> {
+  ): Promise<{ consumerAddress: string; nonce: string; signature: string | null }> {
     if (isAgentSignature(signerOrAuthToken)) {
       return {
         consumerAddress: signerOrAuthToken.consumerAddress,
         nonce: signerOrAuthToken.nonce,
         signature: signerOrAuthToken.signature
       }
     }
     const consumerAddress = await this.getConsumerAddress(signerOrAuthToken)
     const nonce = ((await this.getNonce(nodeUri, consumerAddress, signal)) + 1).toString()
     const signature = await this.getSignature(signerOrAuthToken, nonce, command)
-    if (!signature) throw new Error(`Could not sign command ${command}.`)
     return { consumerAddress, nonce, signature }
   }
 
@@ -1546,7 +1543,9 @@
       throw new Error(
         'Persistent storage operations require a Signer or AgentSignature (nonce/signature).'
       )
     }
-    return this.getSignedCommandParams(nodeUri, signerOrAuthToken, command, signal)
+    const params = await this.getSignedCommandParams(nodeUri, signerOrAuthToken, command, signal)
+    if (!params.signature) throw new Error('Could not sign persistent storage request.')
+    return params as { consumerAddress: string; nonce: string; signature: string }
   }

• [INFO][style] The refactoring of the P2P dialing fallback logic inside getConnection (where circuits and DHTs are systematically traversed and retried) is a massive code-quality and reliability improvement over the previous array iteration model. Great work!

AI automated code review (Gemini 3).

Overall risk: medium

Summary:
The PR successfully refactors the provider modules to support the Multi-Agent Computing Protocol (MCP). Replacing Signer | string with signerOrAuthTokenOrSignature seamlessly accommodates pre-signed operations via AgentSignature, which is excellent for automated multi-agent environments. The introduction of getSignedCommandParams elegantly DRYs up parameter extraction across API requests. However, a logic flaw in the P2P connection dialing completely drops relayed addresses (/p2p-circuit) if the initial direct connection fails, which will cause connectivity issues in NAT/firewall environments. There are also a couple of minor TypeScript return type mismatches introduced by the new helper functions.

Comments:
• [ERROR][bug] When filtering out /p2p-circuit relay addresses to prefer direct connections, the current logic correctly falls back to dialing relayed addresses only if there were zero direct addresses available (dialable.length < 1). However, if direct addresses are found but they fail to connect (e.g., due to strict NAT/firewalls), node.dial throws an error and the function exits, completely abandoning the relayed fallback. You should catch the error and execute the fallback recursive call before throwing.

     try {
       const conn = await node.dial(dialable, { signal })
       LoggerInstance.debug(
         `[P2P] Dial SUCCESS via ${conn.remoteAddr} (limited=${conn.limits != null})`
       )
       return conn
     } catch (err: any) {
+      if (!includeP2PCircuit && afterPFilter < beforePFilter) {
+        LoggerInstance.debug(`[P2P] Direct dial failed, falling back to relayed addresses...`)
+        return this.getConnection(
+          { nodeId: peerId ? peerId.toString() : '', multiaddress: addrs } as NodeP2P,
+          signal,
+          true
+        )
+      }
       throw new Error(
         `Cannot dial peer ${peerId?.toString()}. ` +
           (addrs.length > 0
             ? `Found addrs: ${addrs.map(String).join(', ')}. `
             : 'No addresses found. ') +
           `Active connections: ${node.getConnections().length}. ` +
           err.message
       )
     }

• [WARNING][style] The getSignature method returns Promise<string | null>, but the return type signature for getSignedCommandParams forces signature to be a string. If strictNullChecks is enabled, this will cause a TypeScript compiler error. Update the return type to reflect that signature can be null (e.g., when a JWT auth token string is provided instead of a Signer).

   private async getSignedCommandParams(
     nodeUri: string,
     signerOrAuthToken: signerOrAuthTokenOrSignature,
     command: string,
     signal?: AbortSignal,
     providerEndpoints?: any,
     serviceEndpoints?: any
-  ): Promise<{ consumerAddress: string; nonce: string; signature: string }> {
+  ): Promise<{ consumerAddress: string; nonce: string; signature: string | null }> {

• [WARNING][style] Similar to HttpProvider, the return type of signature should accept null to accommodate the behavior of getSignature.

   private async getSignedCommandParams(
     nodeUri: OceanNode,
     signerOrAuthToken: signerOrAuthTokenOrSignature,
     command: string,
     signal?: AbortSignal
-  ): Promise<{ consumerAddress: string; nonce: string; signature: string }> {
+  ): Promise<{ consumerAddress: string; nonce: string; signature: string | null }> {

• [INFO][other] Removing fetchNodeLogs in favor of expanding downloadNodeLogs to natively handle AgentSignature pre-signed payloads is an excellent refactor. It reduces code duplication and streamlines the API interface. Great design choice.

AI automated code review (Gemini 3).

Overall risk: medium

Summary:
This PR introduces crucial architectural improvements for MCP (Multi-Agent/Machine Communication Protocol) by standardizing authentication and supporting pre-signed payloads via AgentSignature and signerOrAuthTokenOrSignature. The refactoring to centralize signature parameters via getSignedCommandParams is excellent and drastically reduces code duplication. The new multiaddress fallback logic in P2pProvider enhances connection resilience. There are a few TypeScript compilation errors due to mismatched types in persistent storage payloads that need to be addressed, as well as minor stylistic improvements.

Comments:
• [ERROR][bug] getSignedCommandParams returns a Promise where signature can be string | null. However, getPersistentStorageSignaturePayload expects a return type where signature is strictly string. While you explicitly throw if typeof signerOrAuthToken === 'string' (which guarantees signature will be a string at runtime), the TypeScript compiler cannot infer this across function boundaries and will throw a type error (Type 'string | null' is not assignable to type 'string'). You should add a type assertion or explicit check to fix the compilation error.

    const params = await this.getSignedCommandParams(
      nodeUri,
      signerOrAuthToken,
      command,
      signal,
      providerEndpoints,
      serviceEndpoints
    )
    return params as { consumerAddress: string; nonce: string; signature: string }

• [ERROR][bug] Similar to the issue in HttpProvider.ts, returning getSignedCommandParams here directly will cause a TypeScript compilation error because the expected return type requires signature: string, but the function returns signature: string | null.

    const params = await this.getSignedCommandParams(nodeUri, signerOrAuthToken, command, signal)
    return params as { consumerAddress: string; nonce: string; signature: string }

• [INFO][style] The imported getConsumerAddress from BaseProvider.ts already handles the isAgentSignature(s) logic natively. Checking it again here is redundant and can be simplified.

  protected getConsumerAddress(s: signerOrAuthTokenOrSignature) {
    return getConsumerAddress(s)
  }

• [INFO][style] As noted in HttpProvider.ts, the imported getConsumerAddress handles isAgentSignature natively. This redundant check can be removed.

  protected getConsumerAddress(s: signerOrAuthTokenOrSignature) {
    return getConsumerAddress(s)
  }

• [WARNING][style] In TypeScript, standard convention dictates using PascalCase for exported types and interfaces. Consider renaming signerOrAuthTokenOrSignature to SignerOrAuthTokenOrSignature for consistency.

-export type signerOrAuthTokenOrSignature = string | Signer | AgentSignature
+export type SignerOrAuthTokenOrSignature = string | Signer | AgentSignature

• [INFO][style] Calling this.getAuthorization(signerOrAuthToken) twice inside the object literal is slightly redundant and can cause TypeScript strict mode to complain about potentially spreading an undefined value. It's cleaner to assign it to a variable first.

        const authHeader = this.getAuthorization(signerOrAuthToken)
        const response = await fetch(computeStartUrl, {
          method: 'POST',
          body: JSON.stringify(payload),
          headers: {
            'Content-Type': 'application/json',
            ...(authHeader ? { Authorization: authHeader } : {})
          },
          signal
        })

(Note: This pattern occurs in several places throughout HttpProvider.ts)

AI automated code review (Gemini 3).

Overall risk: medium

Summary:
The PR successfully introduces the AgentSignature interface and refactors the HTTP and P2P providers to centralize signing via getSignedCommandParams. This enables decentralized agents (MCP) to proxy signed requests seamlessly. However, there are a few issues to address: a potential breaking change/regression regarding Multiaddr[] arrays, a TypeScript compilation error in strict mode due to unhandled null signatures, and missing deduplication of libp2p addresses.

Comments:
• [WARNING][bug] The isP2pUri function previously checked for Array.isArray(nodeUri) to support Multiaddr[]. Since this check was removed, passing Multiaddr[] will return false, causing getImpl() to return httpProvider. If dropping Multiaddr[] is intentional, ensure this is documented as a breaking change. Otherwise, consider restoring the array check to maintain backward compatibility.

--- a/src/services/providers/BaseProvider.ts
+++ b/src/services/providers/BaseProvider.ts
@@ -74,6 +74,9 @@
       return false
     }
   }
+
+  // Backward compatibility for Multiaddr[]
+  if (Array.isArray(node)) return true
 
   // NodeP2P -> p2p
   if (typeof node === 'object' && ('nodeId' in node || 'multiaddress' in node)) {

• [ERROR][bug] getSignedCommandParams returns a Promise where signature is typed as string | null. However, this method's return type expects signature: string. With strictNullChecks enabled, this will cause a TypeScript compilation error. You should verify the signature exists and typecast the return object.

--- a/src/services/providers/HttpProvider.ts
+++ b/src/services/providers/HttpProvider.ts
@@ -110,14 +110,16 @@
         'Persistent storage operations require a Signer or AgentSignature (nonce/signature).'
       )
     }
-    return this.getSignedCommandParams(
+    const params = await this.getSignedCommandParams(
       nodeUri,
       signerOrAuthToken,
       command,
       signal,
       providerEndpoints,
       serviceEndpoints
     )
+    if (!params.signature) throw new Error('Failed to generate signature for persistent storage.')
+    return params as { consumerAddress: string; nonce: string; signature: string }
   }

• [ERROR][bug] Similar to the HTTP provider, returning this.getSignedCommandParams(...) directly throws a TypeScript error because the signature could theoretically be null according to the type definitions.

--- a/src/services/providers/P2pProvider.ts
+++ b/src/services/providers/P2pProvider.ts
@@ -1561,7 +1561,9 @@
         'Persistent storage operations require a Signer or AgentSignature (nonce/signature).'
       )
     }
-    return this.getSignedCommandParams(nodeUri, signerOrAuthToken, command, signal)
+    const params = await this.getSignedCommandParams(nodeUri, signerOrAuthToken, command, signal)
+    if (!params.signature) throw new Error('Failed to generate signature for persistent storage.')
+    return params as { consumerAddress: string; nonce: string; signature: string }
   }

• [INFO][performance] The previous logic included a Set to deduplicate addresses retrieved from both peerStore and DHT. Removing this means duplicate multiaddrs might be pushed to addrs and subsequently dialed, which can trigger redundant Libp2p dialing efforts. It is highly recommended to deduplicate them before mapping/filtering.

--- a/src/services/providers/P2pProvider.ts
+++ b/src/services/providers/P2pProvider.ts
@@ -403,7 +403,11 @@
         }
       }
     }
-    let dialable = addrs.filter((ma) => this.isDialable(ma))
+    
+    const uniqueAddrs = Array.from(
+      new Map(addrs.map((ma) => [ma.toString(), ma])).values()
+    )
+    let dialable = uniqueAddrs.filter((ma) => this.isDialable(ma))
     const beforePFilter = dialable.length

• [INFO][style] The creation of getSignedCommandParams significantly cleans up repetitive code across all provider command methods. Great use of refactoring to support new authentication schemas without bloat!

AI automated code review (Gemini 3).

Overall risk: medium

Summary:
This Pull Request introduces significant enhancements for Multi-Agent Communication Protocol (MCP) interactions by adding support for AgentSignature, standardizing P2P endpoints with OceanNode, and DRYing up command parameter generation through getSignedCommandParams(). However, there are TypeScript return type mismatches introduced in the persistent storage operations that will break strict builds. The transition generally exhibits excellent architectural design, but care must be taken regarding removed APIs and P2P DHT configurations.

Comments:
• [ERROR][bug] There is a TypeScript return type mismatch. getSignedCommandParams returns a Promise where signature is string | null, but getPersistentStorageSignaturePayload expects the Promise to return signature: string. In TS strict mode, this will cause a compilation error. Consider validating and explicitly casting the result.

-    return this.getSignedCommandParams(
+    const params = await this.getSignedCommandParams(
       nodeUri,
       signerOrAuthToken,
       command,
       signal,
       providerEndpoints,
       serviceEndpoints
     )
+    if (!params.signature) {
+      throw new Error('Could not sign persistent storage request.')
+    }
+    return params as { consumerAddress: string; nonce: string; signature: string }

• [ERROR][bug] Similar to HttpProvider, there is a TypeScript return type mismatch here since getSignedCommandParams can return signature: string | null.

-    return this.getSignedCommandParams(nodeUri, signerOrAuthToken, command, signal)
+    const params = await this.getSignedCommandParams(nodeUri, signerOrAuthToken, command, signal)
+    if (!params.signature) {
+      throw new Error('Could not sign persistent storage request.')
+    }
+    return params as { consumerAddress: string; nonce: string; signature: string }

• [WARNING][bug] If TypeScript strict or useUnknownInCatchVariables mode is enabled, the err variable inside the catch block defaults to unknown. Accessing err.name will result in a compile-time error. Add an explicit any type or check instanceof Error.

-    } catch (err) {
-      if (err.name !== 'AbortError') console.error(err)
+    } catch (err: any) {
+      if (err?.name !== 'AbortError') console.error(err)

• [WARNING][other] The fetchNodeLogs method has been removed entirely in favor of the newly refactored downloadNodeLogs. This constitutes a breaking change for existing API consumers utilizing fetchNodeLogs. Ensure this is documented in the changelog, or consider preserving the method as a deprecated wrapper.
• [INFO][performance] Setting clientMode: false forces the node to act as a DHT server. While this optimizes network queries (valuable for always-on MCP agents), it can consume additional bandwidth and pollute the DHT with unreachable nodes if deployed behind a NAT without a publicly dialable address. Ensure this aligns with the expected consumer environments for ocean.js.
• [INFO][style] The connection fallback logic inside getConnection is excellent. Automatically prioritizing direct dials while gracefully falling back to relayed addresses (p2p-circuit) if none are dialable significantly improves connection robustness.

AI automated code review (Gemini 3).

Overall risk: high

Summary:
This PR introduces support for CompleteSignature and updates provider interfaces to use OceanNode and SignerOrAuthTokenOrSignature, modernizing the API to handle multiple authentication mechanisms. The architectural direction and P2P connection fallback mechanisms are excellent. However, there are critical bugs introduced in getSignedCommandParams that result in consumerAddress evaluating to undefined when using Auth Tokens, which will break request payloads. Additionally, there are missing signature propagation fixes in validateDdo and a potentially missing update for validateDdo in HttpProvider.

Comments:
• [ERROR][bug] When signerOrAuthToken is an auth token string, this block returns { consumerAddress: undefined, ... }. This causes consumerAddress to be undefined in request payloads that rely on it (e.g., computeStart). You should decode the address from the JWT to preserve existing functionality. Also, updating the return type to Promise<Partial<CompleteSignature>> satisfies strict typing.

-    if (typeof signerOrAuthToken === 'string') {
-      return {
-        consumerAddress: undefined,
-        nonce: undefined,
-        signature: undefined
-      }
-    }
+    if (typeof signerOrAuthToken === 'string') {
+      return {
+        consumerAddress: await getConsumerAddress(signerOrAuthToken),
+        nonce: undefined,
+        signature: undefined
+      } as Partial<CompleteSignature>
+    }

• [ERROR][bug] Similar to HttpProvider.ts, when signerOrAuthToken is an auth token, consumerAddress is incorrectly returned as undefined. This breaks requests relying on the consumer address.

-    if (typeof signerOrAuthToken === 'string') {
-      return {
-        consumerAddress: undefined,
-        nonce: undefined,
-        signature: undefined
-      } as any
-    }
+    if (typeof signerOrAuthToken === 'string') {
+      return {
+        consumerAddress: await getConsumerAddress(signerOrAuthToken),
+        nonce: undefined,
+        signature: undefined
+      } as Partial<CompleteSignature>
+    }

• [ERROR][bug] When validating a DDO with an auth token, passing null for signerOrAuthToken prevents the Authorization header from being appended to the P2P message. Pass signerOrAuthToken so sendP2pCommand can extract and send the auth token properly.

-      { ddo, publisherAddress, nonce, signature },
-      null,
+      { ddo, publisherAddress, nonce, signature },
+      signerOrAuthToken,
       signal

• [ERROR][bug] validateDdo was updated in BaseProvider to accept SignerOrAuthTokenOrSignature, but HttpProvider's implementation of validateDdo is missing from this PR. If HttpProvider still strictly expects a Signer object, passing a string auth token will cause runtime errors when it attempts to call .getAddress(). Ensure HttpProvider.validateDdo is also updated to handle the new type.
• [WARNING][bug] If signerOrAuthToken is a string, consumerAddress is entirely omitted from the payload, which may cause the node to reject the request. Refactoring this block to always include consumerAddress restores the old behaviour.

-    const payload: Record<string, any> = {
-      command: PROTOCOL_COMMANDS.COMPUTE_GET_RESULT,
-      jobId,
-      index,
-      offset
-    }
-
-    if (typeof signerOrAuthToken === 'string') {
-      payload.authorization = signerOrAuthToken
-    } else {
-      const { consumerAddress, nonce, signature } = await this.getSignedCommandParams(
-        nodeUri,
-        signerOrAuthToken,
-        PROTOCOL_COMMANDS.COMPUTE_GET_RESULT
-      )
-      payload.nonce = nonce
-      payload.signature = signature
-      payload.consumerAddress = consumerAddress
-    }
+    const { consumerAddress, nonce, signature } = await this.getSignedCommandParams(
+      nodeUri,
+      signerOrAuthToken,
+      PROTOCOL_COMMANDS.COMPUTE_GET_RESULT
+    )
+    const payload: Record<string, any> = {
+      command: PROTOCOL_COMMANDS.COMPUTE_GET_RESULT,
+      jobId,
+      index,
+      offset,
+      consumerAddress
+    }
+    if (typeof signerOrAuthToken === 'string') {
+      payload.authorization = signerOrAuthToken
+    } else {
+      payload.nonce = nonce
+      payload.signature = signature
+    }

• [WARNING][bug] If signerOrAuthToken is passed as a raw hex signature (0x...) without being wrapped in CompleteSignature, typeof signerOrAuthToken === 'string' is true, and it will be passed to decodeJwt(signerOrAuthToken). This will throw an exception since a hex signature is not a valid JWT. Consider explicitly throwing an error for hex strings here or forcing raw signatures to be passed via the CompleteSignature type.
• [INFO][performance] Using typeof (node as any).toString === 'function' is true for almost every JavaScript object since they inherit from Object.prototype. This means peerIdFromString will unnecessarily be called (and catch exceptions) for standard generic objects. Consider checking for specific properties (e.g. node.type === 'Ed25519') or using libp2p's isPeerId utility.
• [INFO][style] The public method fetchNodeLogs has been removed. Make sure this API contract change is intentional and well documented, as it is a breaking change for any downstream dependencies relying on this pre-signed log fetching.
• [INFO][performance] Excellent implementation of connection fallback logic. Prioritizing direct transports and correctly filtering out and falling back to relay (/p2p-circuit) addresses avoids unnecessary latency overhead when a direct route is available.

AI automated code review (Gemini 3).

Overall risk: medium

Summary:
This PR introduces an excellent refactoring to abstract authentication mechanisms via SignerOrAuthTokenOrSignature and centralizes signature logic via getSignedCommandParams. It also improves P2pProvider by adding better connection resolution and graceful fallback to P2P-circuit relays. However, there are a few bugs related to URL query parameter construction that will serialize literal "undefined" strings when AuthTokens are used, and a crash vulnerability in getConsumerAddress when raw hex signatures are provided.

Comments:
• [INFO][style] A great refactoring! Centralizing the nonce and signature generation logic significantly reduces code duplication and potential bugs across the provider endpoints.
• [ERROR][bug] You explicitly allow raw 0x... signature strings here, but getConsumerAddress unconditionally passes any string to decodeJwt(). If a caller passes a raw signature string, getConsumerAddress will crash trying to decode it as a JWT. Since CompleteSignature provides a proper way to pass raw signatures along with their consumer address, you should drop the 0x check here to avoid this crash.

   if (typeof signerOrAuthToken === 'string') {
-    // it's either a signature already (0x..) or a jwt token
-    if (signerOrAuthToken.startsWith('0x')) {
-      return signerOrAuthToken
-    } else return null
+    return null
   }

• [WARNING][bug] Because getSignedCommandParams explicitly returns { nonce: undefined, signature: undefined } for AuthTokens, methods that spread this object into URLSearchParams (e.g. createPersistentStorageBucket) will inadvertently serialize literal "undefined" strings (nonce=undefined&signature=undefined). Omitting the undefined keys entirely prevents this issue.

     if (typeof signerOrAuthToken === 'string') {
-      return {
-        consumerAddress: await getConsumerAddress(signerOrAuthToken),
-        nonce: undefined,
-        signature: undefined
-      }
+      return { consumerAddress: await getConsumerAddress(signerOrAuthToken) } as CompleteSignature
     }

• [WARNING][bug] When using an AuthToken, nonce and signature will be undefined. Concatenating them directly results in the literal strings "undefined" being sent as query parameters, which will cause validation to fail on the provider backend. You should append them conditionally.

     let consumeUrl = downloadUrl
     consumeUrl += `?fileIndex=${fileIndex}`
     consumeUrl += `&documentId=${documentId}`
     consumeUrl += `&transferTxId=${transferTxId}`
     consumeUrl += `&consumerAddress=${consumerAddress}`
-    consumeUrl += `&nonce=${nonce}`
-    consumeUrl += `&signature=${signature}`
+    if (nonce) consumeUrl += `&nonce=${nonce}`
+    if (signature) consumeUrl += `&signature=${signature}`

• [WARNING][bug] Similar to the download method, URLSearchParams.set will convert undefined variables into literal "undefined" strings in the query parameters. Only set these values if they are defined.

     const queryParams = new URLSearchParams()
     queryParams.set('consumerAddress', consumerAddress)
     queryParams.set('jobId', jobId)
     if (agreementId) queryParams.set('agreementId', agreementId)
-    queryParams.set('nonce', nonce)
-    queryParams.set('signature', signature)
+    if (nonce) queryParams.set('nonce', nonce)
+    if (signature) queryParams.set('signature', signature)

• [WARNING][bug] Avoid appending "undefined" to the result URL when nonce and signature are omitted due to AuthToken usage.

     if (!computeResultUrl) return null
     let resultUrl = computeResultUrl
-    resultUrl += `?jobId=${jobId}&index=${index}&consumerAddress=${consumerAddress}&nonce=${nonce}&signature=${signature}`
+    resultUrl += `?jobId=${jobId}&index=${index}&consumerAddress=${consumerAddress}`
+    if (nonce) resultUrl += `&nonce=${nonce}`
+    if (signature) resultUrl += `&signature=${signature}`
     return resultUrl

• [INFO][other] The fallback logic handling p2p-circuit multiaddrs is very cleanly implemented. Filtering out relays for the initial direct connection attempt and recursively falling back to relayed paths ensures robust connectivity.

AI automated code review (Gemini 3).

Overall risk: medium

Summary:
This PR introduces a robust refactor to node interactions, unifying parameter gathering via getSignedCommandParams and adding standard connection interfaces (OceanNode). The P2P network dial fallback logic (direct to relayed connections) is particularly well-designed and will substantially increase connection reliability. Furthermore, conditionally omitting the Authorization header prevents unexpected proxy errors. However, there is a critical logic error in URL construction inside HttpProvider.encrypt that will cause malformed requests, alongside minor type-safety concerns regarding the CompleteSignature assertions.

Comments:
• [ERROR][bug] String concatenating the ?chainId=... parameter directly to a potentially null endpoint URL results in the literal string "null?chainId=...". Since this evaluates as truthy, the if (!path) check on the next line is bypassed, leading to failed network requests. Resolve the base path before appending query params.

-    let path =
-      (this.getEndpointURL(serviceEndpoints, 'encrypt')
-        ? this.getEndpointURL(serviceEndpoints, 'encrypt').urlPath
-        : null) + `?chainId=${chainId}`
+    let path = this.getEndpointURL(serviceEndpoints, 'encrypt')?.urlPath
     if (!path) return null
+    path += `?chainId=${chainId}`

• [WARNING][style] Casting this partial object to CompleteSignature forces the TypeScript compiler to assume nonce and signature are populated strings, which they are not in this block. This weakens type safety and can cause hidden runtime bugs if upstream functions expect string methods. Consider changing the return type of getSignedCommandParams across the providers to Promise<Partial<CompleteSignature> & { consumerAddress: string }> instead.

-      return {
-        consumerAddress: await getConsumerAddress(signerOrAuthToken)
-      } as CompleteSignature
+      return {
+        consumerAddress: await getConsumerAddress(signerOrAuthToken)
+      } as Partial<CompleteSignature> & { consumerAddress: string }

• [WARNING][style] The isAuthToken variable was cleaned up elsewhere (e.g. computeStop) but was left behind here. While the logic technically holds since a CompleteSignature object sets isAuthToken to false, it's safer and more consistent with your refactoring to check for the parameters directly:

-    // Is signer, add signature and nonce
-    if (!isAuthToken) {
+    if (signature) {
       url += `&signature=${signature}`
+    }
+    if (nonce) {
       url += `&nonce=${nonce}`
     }

• [INFO][other] An empty catch block will silently swallow all errors (including AbortError if the signal is aborted, or unexpected network failures). It is highly recommended to at least log the error to debug outputs so network resolution issues aren't completely hidden.

-    } catch (err) {}
+    } catch (err: any) {
+      LoggerInstance.debug(`[P2P] findProviders failed: ${err.message}`)
+    }